Skip to content

fix(sql-editor): allow 10k and 50k rows in pivot queries - #108434

Open
mariusandra wants to merge 3 commits into
masterfrom
fix/bi-pivot-query-limits
Open

mariusandra wants to merge 3 commits into
masterfrom
fix/bi-pivot-query-limits

Conversation

@mariusandra

@mariusandra mariusandra commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The BI editor’s pivot-table limit dropdown only offers 100 and 1k, preventing users from querying larger result sets.

Changes

  • Pivot tables now offer 10k and 50k, matching other BI charts and the query engine’s 50k-row maximum.
  • The selected limit reaches the query instead of silently reverting to 1k.

The existing 10k-cell display guard still warns when a pivot grid is too large to render.

Before After
bi-limits-before bi-limits-after

How did you test this code?

Extended the existing pivot query-generation test across all four limits. The 10k and 50k cases fail against the original code and pass with this fix.

SQL editor tests also verify that restoring a shared URL and switching to pivot mode preserve 50k. Validated the full related-test selection used by CI.

Headless Chromium verified selection and persistence at 1100px and 520px. Frontend typecheck, lint/format, and preflight also passed.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Uses sql-editor-bi-mode.

Automatic notifications

  • Publish to changelog?

Docs update

No existing document covers this BI control.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Codex, GPT-6.

Skills: writing-ui-components, writing-tests, setting-feature-flags-in-storybook, hogli, running-ci-preflight, writing-pr-descriptions, reviewing-with-coderabbit, debugging-ci-failures. Local CodeRabbit review skipped because the CLI is signed out; setup was offered earlier in the session. Open-PR search found no duplicate. Screenshots use an isolated Storybook preview with no customer data.

@mariusandra mariusandra self-assigned this Sep 29, 2026
@trunk-io

trunk-io Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

❌ This pull request was removed from the merge queue because it failed tests. PR #108724 was used for testing. See more details here.

Failed Required Status Conclusion
Visual regression tests pass Failure
  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

Copy link
Copy Markdown
Contributor

Hey @mariusandra! 👋

It looks like your git author email on this PR isn't your @posthog.com address (marius.andra@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit c95089f · box box-da623afc9577 · ready in 851s (push → usable) · build log · rebuilds on every push, torn down on close

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team September 29, 2026 15:03
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Complexity (TypeScript) — 5 functions above the limit (max 38)

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

Function Location Complexity Limit
parseBIEditorState frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:249 38 10
parseBIFieldValue frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:217 15 10
filterExpression frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:462 14 10
buildBIQuery frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:620 14 10
BIEditor frontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsx:105 13 10
✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Bundle size — 🟢 -161 B (-0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 68.96 MiB · 🟢 -161 B (-0.0%)

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.58 MiB · 22 files no change █████████░ 85.8% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.52 MiB · 629 files no change █████████░ 87.4% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.35 MiB · 2,339 files no change █████████░ 88.1% of 8.34 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
854 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
301.8 KiB ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
216.9 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.5 KiB src/lib/api.ts
88.5 KiB src/products.tsx
69.4 KiB src/lib/lemon-ui/icons/icons.tsx
40.1 KiB src/lib/utils/eventUsageLogic.ts
38.7 KiB ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
28.4 KiB src/scenes/scenes.ts
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
301.8 KiB ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
272.3 KiB src/taxonomy/core-filter-definitions-by-group.json
216.9 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.5 KiB src/lib/api.ts
98.8 KiB ../packages/quill/packages/quill/dist/index.js
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
88.5 KiB src/products.tsx

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.16 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.16 MiB · 19 files no change ████░░░░░░ 37.8% of 5.72 MiB
Deferred (lazy) 2.10 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
805.4 KiB dist/toolbar/toolbar-app-5UT2PX3W.css
651.7 KiB dist/toolbar/chunk-chunk-N5H2P45W.js
259.4 KiB dist/toolbar/chunk-chunk-UFQG5EMJ.js
138.3 KiB dist/toolbar/chunk-chunk-6OY66GIY.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-VY4JKXM4.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-MAVTJURD.js
21.0 KiB dist/toolbar/chunk-chunk-JBZQMGNO.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🟢 -2.8 KiB (-0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 947.67 MiB · 🟢 -2.8 KiB (-0.0%)

✅ Playwright — all passed

All tests passed.

View test results →

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 2f978ba0-5426-4112-b83f-500590f9cbac

📥 Commits

Reviewing files that changed from the base of the PR and between 5b59481 and c95089f.

📒 Files selected for processing (1)
  • frontend/src/scenes/data-warehouse/editor/sqlEditorLogic.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The BI editor now offers all row-limit options for pivot tables. Heatmap configurations retain the selected limit instead of capping it at 1,000. Tests verify generated queries for limits of 100, 1,000, 10,000, and 50,000, and verify that URL restoration and chart-type changes retain the configured limit.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to c9508

The editor exposes and retains supported row limits, while unsupported values remain rejected. The reviewed changes show no concrete merge-blocking regression.

Architecture Summary

Architecture risk: 🔵 Low · up to c9508

The change affects 1 system.

Changed systems: frontend

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — frontend (service) was modified; 4 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in frontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsx: Removed the PIVOT_TABLE_QUERY_LIMIT import.
  • observed — Modified behavior in frontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsx: Removed the chart-type-specific limit options: pivot tables previously showed only limits at or below PIVOT_TABLE_QUERY_LIMIT, while other chart types showed all limits.
  • observed — Modified behavior in frontend/src/scenes/data-warehouse/editor/bi/BIEditor.tsx: The row-limit selector now always receives LIMIT_OPTIONS; pivot tables no longer have a restricted option list.
  • observed — Modified behavior in frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.test.ts: The heatmap dimension and chart-settings test now runs for limits 100, 1,000, 10,000, and 50,000, and expects each supplied limit in the generated query. Previously it ran only at 50,000 and expected a query limit of 1,000; the dimension and chart-settings assertions remain.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description covers the problem, user-visible changes, screenshots, testing, feature-flag status, documentation, and agent context. It is mostly complete. It does not explicitly document patch cove…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@trunk-io

trunk-io Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
Scenes-App/SidePanels SidePanelNotebooks smoke-test The test timed out while waiting for a loading indicator or spinner to disappear. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

@github-actions
github-actions Bot requested a deployment to preview-pr-108434 September 29, 2026 16:29 In progress

This branch was successfully deployed

1 active deployment
preview-pr-108434 — c95089fa Deployed Sep 29, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants